test: stop requesting explicit auth scopes#151
test: stop requesting explicit auth scopes#151corpo-iwillspeak wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the test suite to stop requesting explicit OAuth scope values, switching functional tests to pass an audience parsed via parseAudience, and extends functional tests to configure the SDK’s gRPC endpoint via grpcAddress.
Changes:
- Remove
scopefrom unit and functional test authentication configurations; update the auth manager unit test mock expectation accordingly. - Update functional tests to set
audience: parseAudience(process.env.VITE_ATHENA_AUDIENCE)and passgrpcAddressfromVITE_ATHENA_GRPC_ADDRESS. - Fix a functional test assertion to correctly verify
response.classificationsis an array.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/module-import.test.ts | Removes scope from the SDK instantiation used for import/creation verification. |
| tests/unit/main.test.ts | Removes scope from several ClassifierSdk constructor tests. |
| tests/unit/authenticationManager.test.ts | Removes scope from options and updates the clientCredentialsGrant expectation to match the no-scope call shape. |
| tests/functional/main.functional.test.ts | Switches functional tests to audience via parseAudience, adds grpcAddress, and fixes an assertion on classifications. |
| tests/functional/e2e.functional.test.ts | Removes scope from E2E test authentication config (audience remains). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // This is a smoke test. You must have a running gRPC server at localhost:50051 for this to pass. | ||
| // You may want to mock the gRPC client for true unit testing. | ||
| const sdk = new ClassifierSdk({ | ||
| deploymentId: process.env.VITE_ATHENA_DEPLOYMENT_ID, | ||
| affiliate: process.env.VITE_ATHENA_AFFILIATE, | ||
| grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS, |
There was a problem hiding this comment.
The comment says the test requires a gRPC server at localhost:50051, but the SDK is now configured via grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS (and otherwise falls back to the SDK default address). Please update the comment to reflect the actual address source (env var / default) so test setup guidance is accurate.
| // This is a smoke test. You must have a running gRPC server at localhost:50051 for this to pass. | ||
| // You may want to mock the gRPC client for true unit testing. | ||
| const imagePath = __dirname + '/448x448.jpg'; | ||
| const sdk = new ClassifierSdk({ | ||
| deploymentId: process.env.VITE_ATHENA_DEPLOYMENT_ID, | ||
| affiliate: process.env.VITE_ATHENA_AFFILIATE, | ||
| grpcAddress: process.env.VITE_ATHENA_GRPC_ADDRESS, |
There was a problem hiding this comment.
This smoke-test setup comment still references localhost:50051, but the test now uses grpcAddress from VITE_ATHENA_GRPC_ADDRESS (or the SDK default). Please update the comment so it matches how the connection target is actually chosen.
|
Dunno what yo udid the break the build but it seems unrelated to the changes in this PR. |
This pull request updates the authentication configuration for both functional and unit tests by removing the
scopeproperty and replacing it with theaudienceproperty, which is now parsed using theparseAudienceutility. Additionally, it introduces thegrpcAddressparameter in several functional test cases and corrects an assertion in one of the tests.Authentication configuration updates:
Replaced the
scopeproperty withaudience, usingparseAudience(process.env.VITE_ATHENA_AUDIENCE)in allClassifierSdkinstantiations in functional tests (main.functional.test.ts) and removedscopefrom unit tests (main.test.ts,authenticationManager.test.ts,module-import.test.ts). [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Updated the
openidClient.clientCredentialsGrantmock expectation to only includeaudience(notscope) in unit tests (authenticationManager.test.ts).Functional test improvements:
Added the
grpcAddressparameter to allClassifierSdkinstantiations in functional tests to support gRPC connections (main.functional.test.ts). [1] [2] [3] [4] [5]Fixed a test assertion to correctly verify that
response.classificationsis an array instead of a boolean (main.functional.test.ts).Imports:
parseAudienceto the import list inmain.functional.test.tsfor use in parsing the audience value.